Close the active segment before compaction removes source segments - #6

Thanks for tracking this down and for adding a regression test. Losing writes after a direct compact() is a real bug, and closing the active segment before the sources are removed is the right fix.

Two things before this can go in:

1. Failure-path reopen can create a stray segment. rotate_segment_if_needed() increments current_segment to _segment_count before it calls compact(). So when compaction fails from that path, open_segment(current_segment) opens (and creates) segment_<segment_count>.dat, which is outside the range. The caller then moves to the last segment, but that empty file is never cleaned up by finalize_compaction() or clear(), and this happens on the storage-full path. Could you clamp it, e.g. reopen min(current_segment, _segment_count - 1), or otherwise make sure only a valid segment is reopened? The same applies to the success path if the rename in finalize_compaction() fails and returns before resetting current_segment.
2. Power-loss note. The contribution policy asks changes to compaction to explain what happens on a power loss at each step (section 5.5). A couple of lines in the description is enough: for example, that closing the handle happens before the COMPACTING journal is written and that nothing is written between the reopen and clear_journal().

Since maintainer edits aren't enabled on the branch, please push these yourself. Reviews may take a while on my end, but this one is close.

Thanks again for finding this and for the regression test. Closing the active segment inside compact() is the right fix, because it covers direct calls as well as the internal callers.

I'm merging this as-is and will handle the remaining issues myself in a follow-up commit, so you don't need to make any changes. The follow-up will:

• flush the write buffer before closing the active segment;
• reopen a segment on every exit path, including the early return when compact.tmp can't be opened, which currently leaves the store with no writable segment;
• clamp the reopened segment to segment_count - 1, so a failed rotation-triggered compaction (or a failed rename in finalize_compaction()) can no longer create a stray seg<segment_count>.dat;
• write up the power-loss behaviour for each step, as the contribution policy asks for compaction changes.

This also covers #11, which fixed the threshold-triggered path on its own.

Thanks for taking this over the line, and for the follow-up. I've read 84e09a6, bed1195 and d5282e6 against what you listed. The flush before the close, the reopen on every exit path, the clamp to segment_count - 1 and the power-loss notes are all there, and the journal-pending guard in compact() closes the truncation case as well. We've rebuilt our firmware on master with them, and the suite passes on our branches (113/113).

While checking the clamp I found an older edge case in the same area, so I've filed it separately as #17 with a fix in #18. With segment_count == 1, a successful compaction sends new writes to seg1, outside the scanned range, and those records are lost on reload. It isn't caused by #6 or your follow-up (0.1.7 does the same), and the defaults never hit it.

On your question in #11: a T-Beam (ESP32, LittleFS) has run on master + microReticulum for 30 minutes with no "Has open FD", but path-store compaction hadn't triggered in that window yet. I'll confirm there once a node has logged a compaction.